Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@FUNDAMENTAL_TYPE_REFACTORING_PLAN.md`:
- Around line 1-1082: The review flags that the large
"FUNDAMENTAL_TYPE_REFACTORING_PLAN.md" is out-of-scope for this PR (which
targets "missing fields"); remove or relocate the file from this PR and instead
publish it as a separate design artifact (e.g., GitHub Discussion, Issue, ADR
folder, or a dedicated RFC repository) or, if keeping in-tree for visibility,
add a clear top-level note indicating "Proposed RFC / Future Work — not part of
current PR" and convert it to a draft (so it won't be merged as part of this
change); ensure references to symbols like TransactionMetadata, TxResponse,
LedgerEntryResponse, SimulateRequest, and TransactionSchema remain in the new
location and update the PR to only include the intended "missing fields"
changes.

In `@MISSING_FIELDS--DONE.md`:
- Around line 1-244: Remove the MISSING_FIELDS--DONE.md planning document from
the repository (or move its contents into the PR description, issue tracker,
project wiki, or a changelog entry) so only the implementation code and
generated OpenAPI/schema artifacts remain committed; if you need to preserve the
notes, create a concise CHANGELOG entry or link the issue/PR instead, then
commit the deletion and update the PR to reflect the removal.

In `@packages/xrpl/test/integration/requests/ammInfo.test.ts`:
- Around line 46-68: The test checks general AMM fields but misses validating
account-specific LP token info; after fetching amm from ammInfoRes.result, add
an assertion that amm.lp_token equals the expected LP token amount for lpWallet
(use lpWallet.classicAddress / any LP balance value set up in ammPool during
test setup to derive the expected string) — e.g., assert.equal(amm.lp_token,
'<expected_lp_token_value>') using the expected value from your ammPool setup so
the test verifies the account-specific LP token field is returned.

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/xrpl/src/models/methods/nftBuyOffers.ts`:
- Around line 45-53: Add an integration test that verifies the nft_buy_offers
pagination fields are preserved and typed: call the nftBuyOffers method (or
exported function handling nft_buy_offers) against a test instance/fixture
returning a response with limit and marker, assert the returned
NftBuyOffersResponse (or equivalent DTO) exposes limit as a number and marker
with the expected value/type, and update the method's parsing/types if the test
fails; focus assertions on the limit and marker fields to prevent future
contract drift.

In `@packages/xrpl/src/models/methods/nftSellOffers.ts`:
- Around line 18-27: Add an integration test for nftSellOffers that sends a
request including both limit and marker and asserts the client transmits those
fields and correctly parses them from the server response: create a test that
calls the nftSellOffers method (or the higher-level client wrapper that invokes
it) with a specific limit and marker, mock or capture the outgoing request to
assert the request body/query contains limit and marker, return a mock paginated
response including marker and any truncated/limited result set, and assert the
parsed response object exposes the same limit/marker values (and any types used
for marker) so regressions in request/response handling for the nftSellOffers
pagination fields are caught.

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/xrpl/test/integration/requests/accountNFTs.test.ts`:
- Around line 150-151: Remove the redundant assertion inside the conditional:
since the if (response.result.marker !== undefined) already guarantees presence,
delete the assert.isDefined(response.result.marker) line (referencing
response.result.marker and the surrounding if block in accountNFTs.test.ts) so
the test doesn't duplicate checks.

In `@packages/xrpl/test/integration/requests/nftBuyOffers.test.ts`:
- Around line 120-121: The test contains a redundant assertion inside the
conditional that already checks marker: remove the unnecessary
assert.isDefined(response.result.marker) in nftBuyOffers.test.ts so the code
simply branches on if (response.result.marker !== undefined) { ... } without
re-asserting; locate the assertion referencing response.result.marker in the
nftBuyOffers.test.ts test and delete that assert to match the pattern used in
nftSellOffers.test.ts.

In `@packages/xrpl/test/integration/requests/nftSellOffers.test.ts`:
- Around line 111-112: The test contains a redundant assertion: inside the
if-check for response.result.marker !== undefined you call
assert.isDefined(response.result.marker); remove that redundant assert (or
invert logic and replace the if with an assert if you intended a required
value). Update the test around response.result.marker (the conditional using
response.result.marker !== undefined and the call to assert.isDefined) so you
either remove the assert inside the if-block or refactor the if into an assert
that validates presence before using the marker.
- Around line 9-12: Remove the unused imports xrpToDrops and
unixTimeToRippleTime from the import list in nftSellOffers.test.ts (they become
unnecessary once the NFTokenMint fields are fixed); update references so only
required uses remain (confirm sellOfferTx and the "additional sell offer" test
still use xrpToDrops if needed) and delete the two imports (xrpToDrops,
unixTimeToRippleTime) from the import block to eliminate unused-import warnings.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/xrpl/test/integration/requests/nftBuyOffers.test.ts`:
- Around line 51-54: The test currently assigns nftokenID to getNFTokenID(...)
?? 'undefined', which hides failures; change this to fail fast by checking the
return of getNFTokenID (the call inside nftBuyOffers.test.ts that sets
nftokenID) and throw or assert immediately if it is undefined (e.g., throw new
Error or use the test framework's assertion) so the test stops with a clear
error instead of using the literal 'undefined' in later requests.
- Around line 108-130: The test currently sets NFTBuyOffersRequest with limit: 1
but may pass without exercising pagination because only one offer exists; update
the test so pagination is guaranteed by either (A) creating at least two buy
offers for nftokenID in the test setup before calling
testContext.client.request, or (B) changing the request/limit to request fewer
results than exist (e.g., keep limit: 1 and ensure >1 offers exist). Locate the
NFTBuyOffersRequest construction and ensure the setup that creates offers
creates multiple offers for nftokenID (or adjust the limit) so
response.result.marker is present and the pagination branch (the subsequent
request using marker) is actually executed and asserted.

In `@packages/xrpl/test/integration/requests/nftSellOffers.test.ts`:
- Around line 50-53: The test assigns nftokenID using getNFTokenID(...) but
falls back to the literal string 'undefined', which allows a malformed ID to
propagate; replace that fallback with an explicit failure: after calling
getNFTokenID with txResponse.result.meta (TransactionMetadata<NFTokenMint>),
check whether the result is undefined and if so throw or assert (fail the test)
with a clear message referencing getNFTokenID and nftokenID so the test stops
immediately instead of continuing with an invalid ID.
- Around line 96-117: The pagination branch may not run because only one sell
offer exists; update the "with marker field for pagination" test so it
deterministically exercises marker pagination by ensuring multiple offers exist
before calling NFTSellOffersRequest: create and submit at least two sell offers
for the same nftokenID in the test setup (using the same helper/fixture you use
elsewhere to create offers), then perform the request with limit: 1 and assert
response.result.marker is defined before using it to fetch the next page via
testContext.client.request.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/xrpl/test/integration/requests/accountLines.test.ts`:
- Around line 86-90: The test currently treats response.result.limit as
optional; make the assertion deterministic by asserting the field exists and has
the expected type/value before further checks: replace the if-check with
explicit assertions such as asserting response.result has property "limit" (or
assert.exists(response.result.limit)), then
assert.isNumber(response.result.limit) and
assert.isAtMost(response.result.limit, 10); update the assertions around the
variable response (from the accountLines test) so the test fails if the new
limit field is missing.

In `@packages/xrpl/test/integration/requests/accountNFTs.test.ts`:
- Around line 22-33: The pagination test is nondeterministic because only one
NFT is minted in beforeEach (mintTx) so the account_nfts response may not return
a marker; update the setup and test to force pagination deterministically by
creating enough NFTs (e.g., call testTransaction with NFTokenMint multiple times
in beforeEach or within the test) or by setting the request Limit to 1 so a
marker is guaranteed; then remove the conditional skip and assert on the
returned marker handling logic in the accountNFTs test (reference NFTokenMint
mintTx, testTransaction, and the response.marker assertion) so the marker path
is always exercised.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@FUNDAMENTAL_TYPE_REFACTORING_PLAN.md`:
- Around line 299-323: The TransactionSchema index-signature weakens public
typings; make it an internal-only schema type (e.g., rename to
InternalTransactionSchema) and stop using it as the public model. Keep the full
public union Transaction and TransactionBase for external exports, remove or
restrict the `[key: string]: unknown` from any type exported for consumers, and
where runtime-flexible schema is required (for schema generation or validation)
use the new InternalTransactionSchema and only export it from an internal
module; reference TransactionSchema, TransactionBase, Transaction, and
BaseTransaction to locate and change the exported types, and mirror the
xrpl.js/MPTokenMetadata approach by providing long-form explicit public
interfaces while keeping a separate flexible schema for runtime use.
- Around line 1-18: The roadmap in FUNDAMENTAL_TYPE_REFACTORING_PLAN.md proposes
repo-wide breaking refactors (affecting symbols/methods ledger, ledger_entry,
submit_multisigned, tx, simulate and referencing ranges 946-1082) and must be
removed from PR `#3217` and moved into a separate RFC/tracking issue; update the
PR to only include the targeted missing-fields/types fix by extracting or
deleting this document from the branch, create a new issue or RFC that contains
the full plan (linking the new issue from the PR), and ensure any references to
the plan (e.g., PR `#3217`, the listed methods) are replaced with a pointer to the
new tracking issue so the narrow fix PR remains non-breaking and focused.
- Around line 163-172: Document reproducible evidence before asserting
ts-json-schema-generator limitations: run the tool against the specific
conditional types discussed, capture and paste exact error outputs (e.g.,
"Error: Encountered a reference to a missing definition"), record the tested
ts-json-schema-generator and TypeScript versions, include the exact type
definitions that trigger failures and a minimal repro (commands/tsconfig) with
steps to reproduce, and reference these findings in the sections that claim the
generator “cannot evaluate conditional types” and the “Flatten to Union Types”
refactor rationale so the breaking-change decision is empirically justified.